chrome. implement offscreen document to keep service worker alive#209
Conversation
There was a problem hiding this comment.
Pull request overview
This PR implements Chrome's Offscreen API to keep the service worker alive, replacing the previous keep-alive mechanism that used periodic alarm checks. The new approach creates an offscreen document that sends periodic heartbeat messages to the background service worker.
Key Changes:
- Replaced interval-based alarm polling with Chrome Offscreen API for service worker lifecycle management
- Added offscreen document that sends keep-alive messages every 20 seconds
- Added "offscreen" permission to the manifest
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/manifest.json | Added "offscreen" permission required for Chrome Offscreen API |
| src/offscreen.html | Created minimal HTML file to host the offscreen script |
| src/offscreen.ts | Implemented keep-alive logic that sends messages every 20 seconds to background |
| src/background/main.ts | Replaced old keep-alive mechanism with offscreen document creation and message handler |
| vite.config.ts | Added offscreen.html to build inputs for bundling |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed everything up to 712cbab in 1 minute and 47 seconds. Click for details.
- Reviewed
122lines of code in5files - Skipped
0files when reviewing. - Skipped posting
2draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. src/background/main.ts:110
- Draft comment:
Duplicate invocation of setupOffscreen: It's added as listeners on onStartup and onInstalled, then called directly. If intentional, add a clarifying comment; otherwise, consider removing the direct call. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% The comment is asking the author to either clarify or change the code. However, this pattern seems intentional: the direct call ensures immediate setup when the background script loads, while the event listeners handle browser restarts. The function has proper guards to prevent duplicate document creation. This comment is essentially asking the author to "confirm their intention" or "add clarifying comments," which violates the rules. The comment doesn't identify a bug or required code change - it's more of a "this might be confusing" observation. The pattern is actually quite common in browser extensions where you want something to run both immediately and on specific events. The pattern could genuinely be confusing to future maintainers, and a clarifying comment might improve code readability. The comment does suggest a concrete action (adding a comment or removing the call), which could be considered actionable rather than purely speculative. While the pattern might benefit from clarification, the rules explicitly state not to ask the author to confirm their intention or add clarifying comments. The comment uses the phrase "If intentional, add a clarifying comment; otherwise, consider removing" which is exactly the type of "verify/ensure/confirm" language the rules say to avoid. There's no actual bug here - the code works correctly with the guards in place. This comment should be deleted. It's asking the author to either confirm their intention or add a clarifying comment, which violates the rules. The code has proper guards and the pattern is intentional and functional. No actual code change is required.
2. src/offscreen.ts:3
- Draft comment:
The comment is in Chinese; consider using English (or a consistent language) for clarity across the codebase. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50% This is a style/consistency comment about using English instead of Chinese. While language consistency can be valuable, the rules state "Do NOT comment unless there is clearly a code change required" and "Do NOT make comments that are obvious or unimportant." This is more of a stylistic preference rather than a functional issue. The code works fine with Chinese comments. Without seeing the rest of the codebase, I can't verify if there's actually an established pattern of English-only comments. The comment is also somewhat presumptuous about what language should be used - maybe this is a Chinese-language project. I might be missing context about whether this codebase has an established English-only policy. If the entire codebase uses English comments and this is the only Chinese comment, it could be a legitimate consistency issue worth addressing. Even if there is an English-only policy, this falls under "obvious or unimportant" comments. The author can see the comment is in Chinese. This is a stylistic preference that doesn't affect functionality, and without strong evidence of a codebase-wide policy visible in this diff, I should err on the side of deletion per the rules. This comment is about style/language preference rather than a functional code issue. It falls under "obvious or unimportant" comments that don't clearly require a code change. Without strong evidence of a codebase policy in the diff itself, this should be deleted.
Workflow ID: wflow_BI7DONsYuOb4zSim
You can customize 
Tokisaki-Galaxy
force-pushed
the
new-chrome-keepalive
branch
from
712cbab to
14418cf
Compare
|
Have you tested this locally? I keep getting error creating bucket for some reason. Properly set up cors and everything in the server. @BelKed Maybe you can give this a go. |
I have used it normally, and after modification, it works well at least at windows 11 edge(143.0.3650.96). 
|
|
Got it working on the python server. Some cors issues on the rust server, I presume. Seems to be working perfectly.🚀 |
The issues on the Rust server appear to be CORS-related; likely because its security policy doesn't yet permit the chrome-extension:// origin/scheme. Since this has been a long-standing issue affecting many users, I’d appreciate it if we could expedite the merge and release of this fix. Thanks! |
|
The Rust server has hard-coded UUID which only extensions installed from the Chrome Web Store have I'll manually test it in the upcoming days, thank you for the fix 💙 |
BelKed
left a comment
BelKed
left a comment
There was a problem hiding this comment.
Manually tested, seems to be working correctly :)
|
This seems tested and approved, is there a reason it hasn't been merged yet? |
|
@ErikBjare :) |
|
@greptileai review |
Greptile SummaryThis PR replaces the previous alarm-based service-worker keep-alive with Chrome's Offscreen API: a minimal
Confidence Score: 4/5Functional approach is sound but the One P1 finding (incorrect offscreen src/background/main.ts — incorrect Important Files Changed
Sequence DiagramsequenceDiagram
participant SW as Service Worker (main.ts)
participant Chrome as chrome.offscreen API
participant OD as Offscreen Document (offscreen.ts)
SW->>SW: setupOffscreen() on startup/install
SW->>Chrome: hasDocument()?
Chrome-->>SW: false
SW->>Chrome: createDocument({ url: 'src/offscreen.html', reasons: ['BLOBS'] })
Chrome-->>OD: Load offscreen.html → offscreen.ts
loop Every 20 seconds
OD->>SW: runtime.sendMessage({ type: 'KEEP_ALIVE' })
SW-->>OD: { status: 'ok' }
Note over SW: Service worker stays alive
end
Reviews (1): Last reviewed commit: "feat: chrome. implement offscreen docume..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| try { | ||
| await _chrome.offscreen.createDocument({ | ||
| url: 'src/offscreen.html', | ||
| reasons: ['BLOBS'], | ||
| justification: 'Keep service worker alive for heartbeat packets', | ||
| }) |
There was a problem hiding this comment.
reasons value for offscreen document
'BLOBS' requires the offscreen document to use the Blob URL API, but offscreen.ts only sends messages — it never touches the Blob API. Chrome Web Store reviewers check that reasons accurately reflects actual usage; a mismatch can lead to extension rejection. The closest valid reason for a "keep-alive timer" pattern is none of the built-in reasons, so the most defensible alternative is 'WORKERS' (background JS) or the fact that this pattern may simply need to be re-evaluated. At minimum the reason should match actual behavior.
| try { | |
| await _chrome.offscreen.createDocument({ | |
| url: 'src/offscreen.html', | |
| reasons: ['BLOBS'], | |
| justification: 'Keep service worker alive for heartbeat packets', | |
| }) | |
| await _chrome.offscreen.createDocument({ | |
| url: 'src/offscreen.html', | |
| reasons: ['DOM_SCRAPING'], | |
| justification: 'Keep service worker alive by maintaining an active DOM context', | |
| }) |
If none of the available reasons honestly apply (AUDIO_PLAYBACK, BLOBS, CLIPBOARD, DOM_PARSER, DOM_SCRAPING, GEOLOCATION, LOCAL_STORAGE, MATCH_MEDIA, PICTURE_IN_PICTURE, PREFER_OFFLINE, USER_MEDIA, WEB_RTC, WORKERS), consider whether the offscreen document can be given a secondary real purpose (e.g. parsing some DOM) to justify the reason used.
| setInterval(keepAlive, 20 * 1000) | ||
| keepAlive() |
There was a problem hiding this comment.
Messaging every 20 seconds is significantly more frequent than necessary. Chrome terminates idle service workers after ~5 minutes, so a 25–30 second interval still provides ample safety margin with far less overhead and power consumption. The original alarm-based approach used 4 minutes.
| setInterval(keepAlive, 20 * 1000) | |
| keepAlive() | |
| setInterval(keepAlive, 25 * 1000) |
| browser.runtime.onMessage.addListener((message: any) => { | ||
| if (message.type === 'KEEP_ALIVE') { | ||
| return Promise.resolve({ status: 'ok' }) | ||
| } | ||
| return undefined | ||
| }) | ||
|
|
||
| // Initialize on startup and installation | ||
| browser.runtime.onStartup.addListener(setupOffscreen) | ||
| browser.runtime.onInstalled.addListener(setupOffscreen) | ||
|
|
||
| setupOffscreen() |
There was a problem hiding this comment.
onMessage and event-listener registration
The previous implementation wrapped the keep-alive logic in if (import.meta.env.VITE_TARGET_BROWSER === 'chrome'). The new code registers the KEEP_ALIVE message listener and calls setupOffscreen() unconditionally for all browsers. While setupOffscreen() safely no-ops on Firefox/Safari (no chrome.offscreen), the KEEP_ALIVE message listener is now always active. This is harmless today but silently widens scope. Wrapping the entire block in a Chrome check keeps the intent explicit:
if (import.meta.env.VITE_TARGET_BROWSER === 'chrome') {
browser.runtime.onMessage.addListener((message: any) => {
if (message.type === 'KEEP_ALIVE') {
return Promise.resolve({ status: 'ok' })
}
return undefined
})
browser.runtime.onStartup.addListener(setupOffscreen)
browser.runtime.onInstalled.addListener(setupOffscreen)
setupOffscreen()
}Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
https://github.com/Tokisaki-Galaxy/aw-watcher-web/actions
Passed the action compilation and grammar check, and has been tested to be normal.
resolve #199
Important
Implements Offscreen API in Chrome to keep the service worker alive by adding offscreen document and periodic messaging.
setupOffscreen()inmain.tsto keep the service worker alive using the Offscreen API for Chrome.KEEP_ALIVEmessages fromoffscreen.tsto prevent service worker termination.KEEP_ALIVEmessages inmain.tsand responds with status 'ok'.offscreen.htmlto loadoffscreen.ts.offscreen.tsto send periodic messages to the background script.manifest.jsonto includeoffscreenpermission for Chrome.vite.config.tsto includesrc/offscreen.htmlinadditionalInputs.This description was created by
for 712cbab. You can customize this summary. It will automatically update as commits are pushed.